Feat: Funding Pot #741

fabianschu · 2025-04-30T22:36:49Z

No description provided.

src/modules/logicModule/interfaces/ILM_PC_FundingPot_v1.sol

src/modules/logicModule/LM_PC_FundingPot_v1.sol

0xNuggan · 2025-05-04T09:12:34Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        }
+
+        if (block.timestamp > round_.roundStart) {
+            revert Module__LM_PC_FundingPot__RoundAlreadyStarted();


A bit of a nitpick, but since this will also trigger if somebody tries to edit an already finished round, maybe we can change the name (or add an extra error if size allows for it?)

I can't really come up with a more descriptive error since the idea is that the admin can edit as long as a round hasn't started. Imo the error exactly tells this story.

Yeah, fair. My comment came from the thought of "if a round started and ended, it's weird to revert saying 'it started' ". But it makes sense, the condition is "rounds are editable before start". Let's leavi it like that.

src/modules/logicModule/LM_PC_FundingPot_v1.sol

0xNuggan

In general looks good, there are a couple of things I pointed out. I also have still an open question about the caps/accesstypes, but I wanted to get this out first

0xNuggan · 2025-05-23T17:55:25Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        }
+
+        if (block.timestamp > round_.roundStart) {
+            revert Module__LM_PC_FundingPot__RoundAlreadyStarted();


Yeah, fair. My comment came from the thought of "if a round started and ended, it's weird to revert saying 'it started' ". But it makes sense, the condition is "rounds are editable before start". Let's leavi it like that.

omega-audits · 2025-05-25T18:15:06Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+    // Constants
+
+    /// @notice The role that allows creating funding rounds.
+    bytes32 public constant FUNDING_POT_ADMIN_ROLE = "FUNDING_POT_ADMIN";


You will need to update it for the new authorization logic

omega-audits · 2025-05-25T18:17:26Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+    }
+
+    /// @inheritdoc ILM_PC_FundingPot_v1
+    function getRoundAccessCriteria(uint32 roundId_, uint8 accessCriteriaId_)


Should revert if the roundId_ or its accessCriteriaId_ don't exist

(Unless returning all empty over revert is intended)

omega-audits · 2025-05-25T18:28:44Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+    ) external onlyModuleRole(FUNDING_POT_ADMIN_ROLE) {
+        Round storage round = rounds[roundId_];
+
+        if (accessCriteriaType_ > MAX_ACCESS_CRITERIA_ID) {


Here it checks against the type not the id. According to the name and other uses of MAX_ACCESS_CRITERIA_ID, it should both check here against accessCriteriaId_ instead of accessCriteriaType_, and in if accessCriteriaId_ is 0 it should check roundIdToNextAccessCriteriaId won't be higher.

Though it's a bit unclear why require it to be up to 4?
Was that meant to be limiting the type or the ID in fact?

omega-audits · 2025-05-25T18:31:22Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        // Clear all existing data to prevent stale data
+        round.accessCriterias[criteriaId].nftContract = address(0);
+        round.accessCriterias[criteriaId].merkleRoot = bytes32(0);
+        // @note: When changing allowlists, call removeAllowlistedAddresses first to clear previous entries


Shouldn't this be called here then to make sure it's removed?

omega-audits · 2025-05-25T20:28:11Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        _validateEditRoundParameters(round);
+
+        for (uint i = 0; i < addressesToRemove_.length; i++) {
+            unchecked {


What's this unchecked for? (there's no math operation inside here)

omega-audits · 2025-05-25T20:38:28Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+                    Module__LM_PC_FundingPot__UnspentCapsRoundIdsNotStrictlyIncreasing(
+                );
+            }
+


Could move lastSeenRoundId = currentProcessingRoundId; here and delete it from the 3 other places it's done below to avoid code repetition.

omega-audits · 2025-05-25T20:41:36Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+    {
+        Round storage round = rounds[roundId_];
+
+        if (round.roundEnd == 0 && round.roundCap == 0) {


Instead of this, you could just check if the round ID is lower than the current one in the counter (here and in other places this check is done)

Or check roundStart != 0 as it cannot be 0 when a round is added

omega-audits · 2025-05-25T20:46:52Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        uint contributorCount = contributors.length;
+
+        // Check batch size is not zero
+        if (batchSize_ == 0 || batchSize_ > contributorCount) {


It might make operations more convenient to set batchSize_ to contributorCount instead of reverting if it's bigger

omega-audits · 2025-05-25T21:21:28Z

src/modules/logicModule/LM_PC_FundingPot_v1.sol

+        }
+
+        // --- Personal Cap Check ---
+        {


Should have an if here to check if personal cap exists/ should be applied

fabianschu requested a review from 0xNuggan April 30, 2025 22:37

0xNuggan reviewed May 5, 2025

View reviewed changes

0xNuggan requested changes May 5, 2025

View reviewed changes

zzzuhaibmohd and others added 27 commits May 22, 2025 14:46

init: create the funding pot v1 template

a20b087

feat: funding pot admin authorization

fdec788

test: add tests for funding pot contract

5fea6e6

feat: add checks for granting and revoking roles

64fdd6a

feat: add funding pot admin role check

c641ddd

test: add gherkin guide and unit tests

5da9352

fix: add missing internal function to exposed_

a07596a

chore: remove redundant template code

29a6971

test: remove unused test code

307303d

test: code format and unit tests

0afb10c

chore: apply PR review changes

6d2fe12

Test: Add test for internal function

3388d2f

chore: remove redundant code

edadf3a

chore: inverter standard change

2deacdc

Feat: Add round creation and edit functions

03694d1

feat: create and edit rounds implementation

cf292e5

test: add tests for the edit rounds functionality

ce9b04a

fix: follow inverter standard and remove redundant code

a5b4ce7

fix: follow inverter standard and remove redundant code

10cc1ea

fix: follow inverter standard and remove redundant code

b058ca0

feat: add access criteria implementation

23097bf

test: code format & access control criteria unit tests

5962650

fix: inverter standard changes

72a3f18

fix: add bool to getRoundAccessCriteria()

e683568

fix: inverter standard changes

48cedee

fix: inverter standard changes

3179ce5

fix: PR Review Fix#1

9c8e42a

JeffreyJoel and others added 26 commits May 22, 2025 14:49

tests: improve test coverage

d6daec1

chore: code format

a9d9a7f

chore: code format & fix minor bugs

b419bc8

chore: Remove stale/irrelevant values in set/edit roundAccessCriteria

8ac8cb9

chore: add support for multiple access control criteria

398a9fc

fix: fix double underscore styling issue

8ad0cd5

chore: change 'accessId' to 'accessCriteriaId' in the test file

be5acae

chore: convert necessary getter functions from internal to external

57d3a44

fix: add missing assertion for e2e test

3928fc1

fix: add optimization to reduce contract size

099fe3f

feat: accumulation mode

3f1bd06

chore: adds tests for accumulation mode behavior

8682631

feat: globally configurable start for accumulation logic

f36024b

fix: mode-specific logic & tests

c44e206

chore: contract size excess 879 -> 239

f033a53

chore: custom compiler settings for FP

fd065ca

fix: adds missing additional_compiler_profiles

881630c

fix: max personal cap defined by highest access criteria

4145835

chore: test structure cleanup

a943aef

chore: fmt

198f733

chore: removes redundant code

fb1ad69

fix: compiler warnings

e29de85

chore: remove redundant check on zero contribution

240f52e

chore: fmt

2a5371f

fix: >= instead of ==

6d3384b

fix: merge conflicts

8dda04f

fabianschu force-pushed the feat/funding-pot branch from f8d3200 to 8dda04f Compare May 22, 2025 15:32

fabianschu requested a review from 0xNuggan May 22, 2025 15:35

0xNuggan approved these changes May 23, 2025

View reviewed changes

omega-audits reviewed May 25, 2025

View reviewed changes

Feat: Funding Pot #741

Are you sure you want to change the base?

Feat: Funding Pot #741

Conversation

fabianschu commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xNuggan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!